refactor: split ingest.py and llm.py by responsibility (KISS, no behavior change) - #137
Merged
Merged
Conversation
…s modules ingest.py had grown to ~3300 lines. Following the store.py facade pattern, the machinery moves into three sibling modules while ingest.py stays the orchestrator and facade — every ingest._* seam remains addressable: - ingest_scan.py: discovery walk, guarded deletion sweep, source classification and the file/repo partitions - ingest_staging.py: staging copies, snapshot/diff-by-hash, validate and re-stamp, rename-link repair, the robust file primitives and the base-aware promote - ingest_sessions.py: the all-or-nothing session runner, resume-checkpoint glue and large-source pass planning Pure code motion — moved lines are byte-identical, no behavior change. Tests that monkeypatch internals whose call sites moved now patch the module that binds them (ingest_staging/_robust_copy_file and time/os/shutil retry seams, ingest_sessions/_promote). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0174EYZvbAwV9xttVfddYycH
…m_usage.py The passive accounting layer — SessionUsage, combine_usage, the cost/AIC formatters, and the per-backend envelope parsers (claude result envelope, copilot JSONL, agy stream-json) — moves to llm_usage.py. llm.py re-exports every name, so llm.SessionUsage / llm.combine_usage / llm._usage_from_* stay the addressable surface and llm.py remains the only place that talks to an LLM. Pure code motion, no behavior change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0174EYZvbAwV9xttVfddYycH
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0174EYZvbAwV9xttVfddYycH
There was a problem hiding this comment.
Pull request overview
This PR refactors citadel by splitting the previously large ingest.py and llm.py modules into smaller, responsibility-focused sibling modules while preserving the public “facade” import surface (i.e., callers still address the same ingest._* / llm.* seams).
Changes:
- Split ingest responsibilities into
ingest_scan.py(discovery/partitioning),ingest_staging.py(staging + promote primitives), andingest_sessions.py(session runner + resume/chunking planning). - Split passive session-usage accounting out of
llm.pyintollm_usage.py, with re-exports maintained fromllm.py. - Update tests and architecture docs to follow the new module boundaries.
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/test_ingest_staging.py | Updates monkeypatch targets for staging-related internals after ingest split. |
| tests/test_ingest_resume.py | Updates monkeypatch target for _promote to the module that now binds it (ingest_sessions). |
| tests/test_ingest_chunking.py | Updates monkeypatch target for _promote to the module that now binds it (ingest_sessions). |
| CLAUDE.md | Updates architecture documentation to reflect new ingest/llm module layout. |
| citadel/llm.py | Removes accounting implementation details and re-exports them from llm_usage.py. |
| citadel/llm_usage.py | New module containing passive accounting datatypes and parsers previously in llm.py. |
| citadel/ingest_staging.py | New module for staging/snapshot/promote and robust file primitives. |
| citadel/ingest_sessions.py | New module for running agent sessions (including resume + chunking planning). |
| citadel/ingest_scan.py | New module for discovery walk, deletion sweep guards, and source partitioning. |
| .github/copilot-instructions.md | Regenerated instructions to match the updated architecture text. |
Comments suppressed due to low confidence (2)
tests/test_ingest_staging.py:151
- This rollback test patches
ingest_staging.shutil.rmtree/ingest_staging.time.sleep, but the code path under test usesconfig.robust_rmtree()(and thereforeconfig.shutil/config.time). Patch theconfigmodule so the simulated undeletable share behavior is actually exercised.
monkeypatch.setattr(ingest_staging.shutil, "rmtree", undeletable_share)
monkeypatch.setattr(ingest_staging.time, "sleep", lambda *_: None)
tests/test_ingest_staging.py:418
- Same issue as earlier:
_robust_rmtreegoes throughconfig.robust_rmtree(), so patchingingest_staging.shutil.rmtree/ingest_staging.time.sleepwon’t influence staging cleanup behavior. Patchconfig.shutil.rmtreeandconfig.time.sleepto make the simulated share flakiness take effect.
monkeypatch.setattr(ingest_staging.shutil, "rmtree", flaky_rmtree)
monkeypatch.setattr(ingest_staging.time, "sleep", lambda *_: None)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
The two largest modules had outgrown their files —
ingest.pyat ~3,300 lines (2.5× the next largest) andllm.pyat ~1,260. This PR splits them by responsibility, following the existingstore.pyfacade pattern: pure code motion, moved lines byte-identical, no behavior change.ingest.py(3,297 → 1,331 lines) — orchestrator + facade over three new siblingsingest_scan.pyingest_staging.pyingest_sessions.pyingest.py(kept)IngestReport, the shared_SourceJobloop (serial +--jobs N),retry_candidates,ingest()/_ingest_runEvery externally-addressed seam (
ingest._promote,ingest._run_agent_sessions,ingest._candidates, … — used by tests,status.py,curate.py) is re-exported throughingest, so the module boundary stays an implementation detail.llm.py(1,263 → 938 lines) — passive accounting split intollm_usage.py(371)SessionUsage,combine_usage, the cost/AIC formatters, and the per-backend envelope parsers (claude result envelope, copilot JSONL, agy stream-json) move tollm_usage.py, fully re-exported.llm.pyremains the only place that talks to an LLM.Test + docs adjustments
ingest_staging._robust_copy_file+ thetime/os/shutilretry seams,ingest_sessions._promote). No assertion changed.CLAUDE.mdarchitecture notes updated;.github/copilot-instructions.mdregenerated via the drift guard.Verification
uv run pytest -q— 1302 passed, 1 skipped (whole offline suite)uv run ruff check ./uv run ruff format --check .— cleancitadel lint— cleandef/classcounts identical before/after both splits (77 for ingest, 42 for llm) — nothing lost in the motionverify-corpus(needs a logged-in agent CLI; not available in this environment). Since this is pure code motion with the moved lines byte-identical and the full offline suite green, ingest/llm behavior is unchanged by construction — but a corpus run before release wouldn't hurt.🤖 Generated with Claude Code
https://claude.ai/code/session_0174EYZvbAwV9xttVfddYycH
Generated by Claude Code